Look up cluster_uuid#857
Conversation
|
@cachedout I've never built an LS plugin and tested it in conjunction with another LS core PR. Would you mind posting some instructions on how I can essentially run this PR along with elastic/logstash#10561? Thanks! |
|
@ycombinator I did my development by editing the installed gem directly, the instructions here should allow for testing in your environment after checking out this PR into a local branch: https://github.com/logstash-plugins/logstash-output-elasticsearch/blob/master/README.md#2-running-your-unpublished-plugin-in-logstash |
yaauie
left a comment
There was a problem hiding this comment.
After several read-throughs, it appears as though this and the other PR accomplish the goal, but do so in a way that is fragile and bound to surprise developers in the future, exploiting Settings as a key/value store by means of dynamically creating settings with specific default values that are otherwise unreachable.
I realise that it can be frustrating to try to wire features through multiple components of our ecosystem, and would like to support you in any way that I can to reduce that frustration. I will follow up momentarily with a PR on Logstash that would add a per-plugin metadata store that could be used to this effect, along with instructions for its use.
|
Relevant PR on Logstash core is elastic/logstash#10636 With it, the implementation of def discover_cluster_uuid
if defined?(plugin_metadata)
cluster_info = @client.get('/')
plugin_metadata.set(:cluster_uuid, cluster_info['cluster_uuid'])
end
endAnd extracting this metadata in your other PR becomes something like: def resolve_cluster_uuids
outputs.each_with_object(Set.new) do |output, cluster_uuids|
if LogStash::PluginMetadata.exists?(output.id)
cluster_uuids << LogStash::PluginMetadata.for_plugin(output.id).get(:cluster_uuid)
end
end.to_a.compact
end |
|
jenkins, re-test this |
46748e5 to
b7b3ef2
Compare
This reverts commit b7b3ef2.
The DLQ tests were written to just assume a clean buffer in the logs but this is not a safe assumption. Now we temporarily replace the logging subsystem with a double during these tests so that we can be assured that we are checking a clean instance.
|
The failing tests also appear to be failing on the master branch for me. I had to also fix up some other tests which were failing because they depended on a certain sequence of log events instead of being fully isolated. @yaauie can you re-review this? |
|
Changes pushed. This is still failing a local test but I believe this is a problem unrelated to this PR. It also fails in the master branch and is due to a problem that occurs when attempting to fetch the ES version with a path specified. As such, I believe this is ready for merge. |
One of two PRs necessary to address elastic/logstash#10602